feat(osv-advisory-source,validate-url): add SSRF defense for --osv-url flag#751
feat(osv-advisory-source,validate-url): add SSRF defense for --osv-url flag#751luojiyin1987 wants to merge 10 commits into
Conversation
- Use dns.lookup with { all: true } to check ALL resolved addresses
- Reject if ANY resolved address is private/reserved
- Expand PRIVATE_IP_PATTERNS to cover:
- CGNAT (100.64.0.0/10)
- IETF Protocol (192.0.0.0/24)
- Documentation addresses (TEST-NET-1/2/3)
- Benchmark (198.18.0.0/15)
- Reserved (240.0.0.0/4)
- Broadcast (255.255.255.255)
- IPv6 unspecified (::)
- IPv6 documentation (2001:db8::/32)
- IPv6 NAT64 (64:ff9b::/96)
- IPv6 discard (100::/64)
- IPv6 Teredo (2001::/32)
- IPv6 benchmark (2001:2::/48)
- Add IPv4-mapped IPv6 address detection (::ffff:127.0.0.1)
- Add comprehensive DNS resolution tests
bd2a1d0 to
d4774c4
Compare
sonukapoor
left a comment
There was a problem hiding this comment.
Good work on the layered approach — HTTPS-only plus DNS resolution plus explicit bypass is the right model for this. Two bugs in the IP pattern table need fixing before this can merge, plus a few smaller things.
Also: --allow-private-osv-url without --osv-url is silently accepted right now. A quick guard like if (options.allowPrivateOsvUrl && !options.osvUrl) with a clear error message would make the pairing explicit.
In website/docs/security-assurance-case.md, the A10 row opens with "The CLI only makes outbound HTTP requests to fixed, hardcoded endpoints" but then describes user-controlled URL behavior — those two halves contradict each other. The old wording was accurate before --osv-url existed. Worth leading with the user-controlled case and describing the mitigations directly, dropping the "fixed, hardcoded" claim.
- Fix CGNAT regex gap: second-octets 108,109,118,119 now blocked - Add IPv4 multicast pattern (224.0.0.0/4) to private IP check - Guard --allow-private-osv-url without --osv-url with clear error - Replace console.warn with return-value warning to avoid stderr bleed in --json mode - Add test cases for CGNAT edge cases, multicast, and allowPrivate guard
…ation - Store ssrfWarning instead of returning early, so all subsequent guards (--fix --json, --create-pr, --report --json, --ca-cert) still run when --allow-private-osv-url is set - Add 5 regression tests covering invalid flag combos with private --osv-url
Summary
Adds Server-Side Request Forgery (SSRF) protection for the
--osv-urlflag to prevent abuse when users specify custom OSV endpoints.Fixes #750
Problem
The
--osv-urlflag currently accepts any URL that passesnew URL()parseability. An attacker who controls this flag could redirect outbound HTTP traffic to internal services, exfiltrating package data from lockfiles.Solution
Implement multi-layered SSRF defense:
--allow-private-osv-urlflag to bypass protection for trusted internal mirrorsredirect: 'error'on all fetch calls to prevent redirect-based attacksChanges
Core Implementation
src/utils/validate-url.ts(new) - URL validation with HTTPS check, DNS resolution, private IP detectionsrc/types.ts- AddedallowPrivateOsvUrl?: booleantoParsedOptionssrc/cli/args.ts- Parse--allow-private-osv-urlflagsrc/cli/validate.ts- CallvalidateOsvUrl()in validationsrc/index.ts- Updated to asyncvalidateOptions()withawaitsrc/advisory/osv-advisory-source.ts- Addedredirect: 'error'to both fetch callsTests
tests/validate-url.test.ts(new) - 22 test cases covering HTTPS validation, DNS resolution, private IP detectiontests/validate.test.ts- Updated to async patterntests/osv-advisory-source.test.ts- Updated fetch mock expectationsDocumentation
README.md- Added usage exampleswebsite/docs/cli-reference.md- Added flag documentation with security notewebsite/docs/corporate-proxy.md- Added SSRF protection guide for internal mirrorswebsite/docs/offline-advisory-db.md- Added flag documentationwebsite/docs/security-assurance-case.md- Updated A10 SSRF statusSecurity Considerations
api.osv.dev) allowed without--osv-url--osv-url: Requires HTTPS and public IPs only--allow-private-osv-url: Explicitly bypasses protection for trusted internal mirrorsTesting
Example Usage
This PR addresses Finding 2 from the security audit report.